Hi @yortus, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

Can I get some help with the tests? There's one failing conformance test (tests/cases/conformance/expressions/typeGuards/typeGuardsWithInstanceOfByConstructorSignature.ts). How do I fix that? Is that a case accepting new baselines?

Also should I add any new tests for code coverage with this change?

Hey @yortus looks like the codebase isn't even compiling according to the CI. I wonder what that's about. Make sure you're up to date with master and try compiling locally. We've made some changes to Map recently.

As for the tests, yes, if the baselines look acceptable (some files may be removed, others may be created), it's a matter of running jake baseline-accept (which entirely replaces the tests/baselines/reference folder with tests/baselines/local).

I recommend a diffing tool that does a good job with folder diffs. Beyond Compare is our tool of choice, but WinMerge has also worked pretty great.

You should also add tests for this change. Specifically, you should add tests that actually demonstrate

• an instanceof check with Object for something of type any.
• an instanceof check with Function for something of type any.
• a type predicate check with Object for something of type any.
• a type predicate check with Function for something of type any.
• a type predicate check with {} for something of type any.

I'd put the tests in either tests/cases/conformance/types/any or tests/cases/conformance/types/narrowing (which would be a new folder).

I'd then name them something like instanceofFunctionObjectNarrowngOnAny1.ts and typePredicateFunctionObjectNarrowingOnAny1.ts. The first test might look like

instanceofFunctionObjectNarrowingOnAny.ts

declare var x: any;

if (x instanceof Function) {
    x();
    x(1, 2, 3);
    x("hello!");
    x.prop;
}

if (x instanceof Object) {
    x.method();
    x();
}

Of course, if you think other tests would be appropriate, it might be a decent idea to add some (e.g. one of the original complaints was that you couldn't narrow from a catch-clause. Probably a good idea to actually show that that works now).

Thanks @DanielRosenwasser, I'll work on the tests.

The CI failure is strange. My branch is up-to-date with the latest master and builds fine locally. The CI error message relates to the Map changes, specifically:

scripts/tslint/preferConstRule.ts(175,15): error TS2322: Type '{}' is not assignable to type 'Map<DeclarationUsages>'.

I haven't touched that file, but I can see it hasn't been updated along with the other Map updates, i.e. the error line is still const names: ts.Map<DeclarationUsages> = {}; in master, but shouldn't it have been updated as part of #10270 to something like const names = ts.createMap<DeclarationUsages>(); as it has been elsewhere?

But I shouldn't be changing that in this PR. Any idea what's happening?

This is potentially because we rely on a nightly of TSLint, which relies on a nightly from us. Though I'm not sure why my PR didn't have the same issue. In any case, check out #10323.

I merged in the change, you can feel free to sync up with master now.

@DanielRosenwasser I've added the tests, so this PR should be ready now.

Now there's another strange build error, but only for node 0.10. From the log:

> jake tests
sh: 1: jake: not found
npm ERR! Test failed.  See above for more details.

Any insights?

Not sure why that's happening; seems totally unrelated. We might need to contact Travis CI support.

I'm just gonna close and re-open this PR to re-trigger the CI build, to rule out some intermittent factor.

Hi @yortus, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

Nope, same failure on node 0.10...

The Node 0.10 tests fails to find jake but isn't installed during the npm install step of the build.
This appears to be because the cache setting in .travis.yml caches node_modules/ but this cache seems to be invalid for some reason.

I've set up the CI on my fork of this PR which succeeds when all packages are installed.

The remedy here seems to be to clear the cache which someone on the TypeScript team needs to do.

Cleared the cache and restarted the build. Fingers crossed!

Seems to have failed again 😡.
Next possible solutions.. In the .travis.yml in this branch/PR:

1. remove the cache config, commit and build then
2. undo the changes, commit and build again
3. profit

After that I guess the commits can just be removed using git reset --hard and push with --force

Alternatively just create a new PR and maybe this bad dream will all go away.

I'll try opening a new PR.

New PR at #10334 builds successfully.

Sorry about whatever that was. 😕 as well here